lib/pull: Tweak update_timeout logic again

I was hitting `SIGSEGV` when running `cosa build` and narrowed it down
to #1954. What's happening here is that because we're using the default
context, when we unref it in the out path, it may not actually destroy
the `GSource` if it (the context) is still ref'ed elsewhere. So then,
we'd still get events from it if subsequent operations iterated the
context.

This patch is mostly a revert of #1954, except that we still keep a ref
on the `GSource`. That way it is always safe to destroy it afterwards.
(And I've also added a comment to explain this better.)

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index dd4dbff1089057bbad319d1a04bdf15e0993b5f2..586b34fe3be6ddd63cafc6782de7b3c1f996aaf8 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -3573,6 +3573,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   g_autofree char **refs_to_fetch = NULL;
   g_autoptr(GVariantIter) collection_refs_iter = NULL;
   g_autofree char **override_commit_ids = NULL;
+  g_autoptr(GSource) update_timeout = NULL;
   gboolean opt_gpg_verify_set = FALSE;
   gboolean opt_gpg_verify_summary_set = FALSE;
   gboolean opt_collection_refs_set = FALSE;
@@ -3674,6 +3675,10 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
     pull_data->async_error = &pull_data->cached_async_error;
   else
     pull_data->async_error = NULL;
+
+  /* Note we're using the thread default (or global) context here, so it may outlive the
+   * OtPullData object if there's another ref on it. Thus, always detach/destroy sources
+   * local to the `ostree_repo_pull*` operation rather than trying to transfer ownership. */
   pull_data->main_context = g_main_context_ref_thread_default ();
   pull_data->flags = flags;
 
@@ -4504,8 +4509,6 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
 
   if (pull_data->progress)
     {
-      g_autoptr(GSource) update_timeout = NULL;
-
       /* Setup a custom frequency if set */
       if (update_frequency > 0)
         update_timeout = g_timeout_source_new (pull_data->dry_run ? 0 : update_frequency);
@@ -4732,6 +4735,8 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   if (!inherit_transaction)
     ostree_repo_abort_transaction (pull_data->repo, cancellable, NULL);
   g_main_context_unref (pull_data->main_context);
+  if (update_timeout)
+    g_source_destroy (update_timeout);
   g_strfreev (configured_branches);
   g_clear_object (&pull_data->fetcher);
   g_clear_pointer (&pull_data->extra_headers, (GDestroyNotify)g_variant_unref);